From: Keir Fraser Date: Tue, 6 May 2008 12:32:18 +0000 (+0100) Subject: SVM: clean up __get_instruction_length_from_list() X-Git-Tag: archive/raspbian/4.8.0-1+rpi1~1^2~14215^2~67 X-Git-Url: https://dgit.raspbian.org/%22http:/www.example.com/cgi/%22https://%22%22/%22http:/www.example.com/cgi/%22https:/%22%22?a=commitdiff_plain;h=ea1c7a7225787dcda405a1a75a1e982624f1ef1a;p=xen.git SVM: clean up __get_instruction_length_from_list() Remove unused arguments, fix its behaviour near page boundaries, inject appropriate pagefaults, and inject #GP if the instruction is not decodable or %eip is not pointing to valid RAM. Signed-off-by: Tim Deegan Signed-off-by: Keir Fraser --- diff --git a/xen/arch/x86/hvm/svm/emulate.c b/xen/arch/x86/hvm/svm/emulate.c index 8d0f0fc652..34afb8733d 100644 --- a/xen/arch/x86/hvm/svm/emulate.c +++ b/xen/arch/x86/hvm/svm/emulate.c @@ -29,18 +29,6 @@ #define MAX_INST_LEN 15 -static int inst_copy_from_guest( - unsigned char *buf, unsigned long guest_eip, int inst_len) -{ - struct vmcb_struct *vmcb = current->arch.hvm_svm.vmcb; - uint32_t pfec = (vmcb->cpl == 3) ? PFEC_user_mode : 0; - if ( (inst_len > MAX_INST_LEN) || (inst_len <= 0) ) - return 0; - if ( hvm_fetch_from_guest_virt_nofault(buf, guest_eip, inst_len, pfec) ) - return 0; - return inst_len; -} - static unsigned int is_prefix(u8 opc) { switch ( opc ) @@ -73,12 +61,7 @@ static unsigned long svm_rip2pointer(struct vcpu *v) return p; } -/* - * Here's how it works: - * First byte: Length. - * Following bytes: Opcode bytes. - * Special case: Last byte, if zero, doesn't need to match. - */ +/* First byte: Length. Following bytes: Opcode bytes. */ #define MAKE_INSTR(nm, ...) static const u8 OPCODE_##nm[] = { __VA_ARGS__ } MAKE_INSTR(INVD, 2, 0x0f, 0x08); MAKE_INSTR(WBINVD, 2, 0x0f, 0x09); @@ -101,70 +84,90 @@ static const u8 *opc_bytes[INSTR_MAX_COUNT] = [INSTR_INT3] = OPCODE_INT3 }; +static int fetch(struct vcpu *v, u8 *buf, unsigned long addr, int len) +{ + uint32_t pfec = (v->arch.hvm_svm.vmcb->cpl == 3) ? PFEC_user_mode : 0; + + switch ( hvm_fetch_from_guest_virt(buf, addr, len, pfec) ) + { + case HVMCOPY_okay: + return 1; + case HVMCOPY_bad_gva_to_gfn: + /* OK just to give up; we'll have injected #PF already */ + return 0; + case HVMCOPY_bad_gfn_to_mfn: + default: + /* Not OK: fetches from non-RAM pages are not supportable. */ + gdprintk(XENLOG_WARNING, "Bad instruction fetch at %#lx (%#lx)\n", + (unsigned long) guest_cpu_user_regs()->eip, addr); + hvm_inject_exception(TRAP_gp_fault, 0, 0); + return 0; + } +} + int __get_instruction_length_from_list(struct vcpu *v, - enum instruction_index *list, unsigned int list_count, - u8 *guest_eip_buf, enum instruction_index *match) + enum instruction_index *list, unsigned int list_count) { struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb; unsigned int i, j, inst_len = 0; - int found = 0; enum instruction_index instr = 0; - u8 buffer[MAX_INST_LEN]; - u8 *buf; + u8 buf[MAX_INST_LEN]; const u8 *opcode = NULL; + unsigned long fetch_addr; + unsigned int fetch_len; + + /* Fetch up to the next page break; we'll fetch from the next page + * later if we have to. */ + fetch_addr = svm_rip2pointer(v); + fetch_len = min_t(unsigned int, MAX_INST_LEN, + PAGE_SIZE - (fetch_addr & ~PAGE_MASK)); + if ( !fetch(v, buf, fetch_addr, fetch_len) ) + return 0; - if ( guest_eip_buf ) + while ( (inst_len < MAX_INST_LEN) && is_prefix(buf[inst_len]) ) { - buf = guest_eip_buf; - } - else - { - if ( inst_copy_from_guest(buffer, svm_rip2pointer(v), MAX_INST_LEN) - != MAX_INST_LEN ) - return 0; - buf = buffer; + inst_len++; + if ( inst_len >= fetch_len ) + { + if ( !fetch(v, buf + fetch_len, fetch_addr + fetch_len, + MAX_INST_LEN - fetch_len) ) + return 0; + fetch_len = MAX_INST_LEN; + } } for ( j = 0; j < list_count; j++ ) { instr = list[j]; opcode = opc_bytes[instr]; - ASSERT(opcode); - - while ( (inst_len < MAX_INST_LEN) && - is_prefix(buf[inst_len]) && - !is_prefix(opcode[1]) ) - inst_len++; - - ASSERT(opcode[0] <= 15); /* Make sure the table is correct. */ - found = 1; - for ( i = 0; i < opcode[0]; i++ ) + for ( i = 0; (i < opcode[0]) && ((inst_len + i) < MAX_INST_LEN); i++ ) { - /* If the last byte is zero, we just accept it without checking */ - if ( (i == (opcode[0]-1)) && (opcode[i+1] == 0) ) - break; + if ( (inst_len + i) >= fetch_len ) + { + if ( !fetch(v, buf + fetch_len, + fetch_addr + fetch_len, + MAX_INST_LEN - fetch_len) ) + return 0; + fetch_len = MAX_INST_LEN; + } if ( buf[inst_len+i] != opcode[i+1] ) - { - found = 0; - break; - } + goto mismatch; } - - if ( found ) - goto done; + goto done; + mismatch: ; } - printk("%s: Mismatch between expected and actual instruction bytes: " - "eip = %lx\n", __func__, (unsigned long)vmcb->rip); + gdprintk(XENLOG_WARNING, + "%s: Mismatch between expected and actual instruction bytes: " + "eip = %lx\n", __func__, (unsigned long)vmcb->rip); + hvm_inject_exception(TRAP_gp_fault, 0, 0); return 0; done: inst_len += opcode[0]; ASSERT(inst_len <= MAX_INST_LEN); - if ( match ) - *match = instr; return inst_len; } diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c index c52c1f5b2b..d1ebd2a220 100644 --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -84,7 +84,10 @@ static void inline __update_guest_eip( { struct vcpu *curr = current; - if ( unlikely((inst_len == 0) || (inst_len > 15)) ) + if ( unlikely(inst_len == 0) ) + return; + + if ( unlikely(inst_len > 15) ) { gdprintk(XENLOG_ERR, "Bad instruction length %u\n", inst_len); domain_crash(curr->domain); @@ -907,8 +910,7 @@ static void svm_vmexit_do_cpuid(struct cpu_user_regs *regs) { unsigned int eax, ebx, ecx, edx, inst_len; - inst_len = __get_instruction_length(current, INSTR_CPUID, NULL); - if ( inst_len == 0 ) + if ( (inst_len = __get_instruction_length(current, INSTR_CPUID)) == 0 ) return; eax = regs->eax; @@ -1083,13 +1085,15 @@ static void svm_do_msr_access(struct cpu_user_regs *regs) if ( vmcb->exitinfo1 == 0 ) { + if ( (inst_len = __get_instruction_length(v, INSTR_RDMSR)) == 0 ) + return; rc = hvm_msr_read_intercept(regs); - inst_len = __get_instruction_length(v, INSTR_RDMSR, NULL); } else { + if ( (inst_len = __get_instruction_length(v, INSTR_WRMSR)) == 0 ) + return; rc = hvm_msr_write_intercept(regs); - inst_len = __get_instruction_length(v, INSTR_WRMSR, NULL); } if ( rc == X86EMUL_OKAY ) @@ -1101,8 +1105,7 @@ static void svm_vmexit_do_hlt(struct vmcb_struct *vmcb, { unsigned int inst_len; - inst_len = __get_instruction_length(current, INSTR_HLT, NULL); - if ( inst_len == 0 ) + if ( (inst_len = __get_instruction_length(current, INSTR_HLT)) == 0 ) return; __update_guest_eip(regs, inst_len); @@ -1125,10 +1128,13 @@ static void svm_vmexit_do_invalidate_cache(struct cpu_user_regs *regs) enum instruction_index list[] = { INSTR_INVD, INSTR_WBINVD }; int inst_len; + inst_len = __get_instruction_length_from_list( + current, list, ARRAY_SIZE(list)); + if ( inst_len == 0 ) + return; + svm_wbinvd_intercept(); - inst_len = __get_instruction_length_from_list( - current, list, ARRAY_SIZE(list), NULL, NULL); __update_guest_eip(regs, inst_len); } @@ -1204,7 +1210,8 @@ asmlinkage void svm_vmexit_handler(struct cpu_user_regs *regs) if ( !v->domain->debugger_attached ) goto exit_and_crash; /* AMD Vol2, 15.11: INT3, INTO, BOUND intercepts do not update RIP. */ - inst_len = __get_instruction_length(v, INSTR_INT3, NULL); + if ( (inst_len = __get_instruction_length(v, INSTR_INT3)) == 0 ) + break; __update_guest_eip(regs, inst_len); domain_pause_for_debugger(); break; @@ -1281,8 +1288,7 @@ asmlinkage void svm_vmexit_handler(struct cpu_user_regs *regs) break; case VMEXIT_VMMCALL: - inst_len = __get_instruction_length(v, INSTR_VMCALL, NULL); - if ( inst_len == 0 ) + if ( (inst_len = __get_instruction_length(v, INSTR_VMCALL)) == 0 ) break; HVMTRACE_1D(VMMCALL, v, regs->eax); rc = hvm_do_hypercall(regs); diff --git a/xen/include/asm-x86/hvm/svm/emulate.h b/xen/include/asm-x86/hvm/svm/emulate.h index cf3d4da86b..eee7831d09 100644 --- a/xen/include/asm-x86/hvm/svm/emulate.h +++ b/xen/include/asm-x86/hvm/svm/emulate.h @@ -34,15 +34,12 @@ enum instruction_index { }; int __get_instruction_length_from_list( - struct vcpu *v, - enum instruction_index *list, unsigned int list_count, - u8 *guest_eip_buf, enum instruction_index *match); + struct vcpu *v, enum instruction_index *list, unsigned int list_count); -static inline int __get_instruction_length(struct vcpu *v, - enum instruction_index instr, u8 *guest_eip_buf) +static inline int __get_instruction_length( + struct vcpu *v, enum instruction_index instr) { - return __get_instruction_length_from_list( - v, &instr, 1, guest_eip_buf, NULL); + return __get_instruction_length_from_list(v, &instr, 1); } #endif /* __ASM_X86_HVM_SVM_EMULATE_H__ */